Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

eve/log: FTP logging #3827

Closed
wants to merge 2 commits into from
Closed

eve/log: FTP logging #3827

wants to merge 2 commits into from

Conversation

jlucovsky
Copy link
Contributor

This changeset is for issue 2315 and implements eve logging for FTP.

FTP logging adds 2 event types:

  1. ftp
  2. ftp_data

Each ftp event contains:

  • command
  • command_data
  • reply (in array format to handle multi-line replies)
  • completion_code

ftp_data events contain:

  • filename
  • command associated with the data transfer.

This PR is based on the RFC pr #3822.

Sample ftp event_type output

{
  "timestamp": "2019-03-10T15:20:21.104835-0700",
  "flow_id": 274790086810710,
  "pcap_cnt": 125,
  "event_type": "ftp",
  "src_ip": "2a01:0e34:ee97:b130:8c3e:45ea:5ac6:e301",
  "src_port": 38970,
  "dest_ip": "2001:0700:0100:0118:0000:0000:0000:0047",
  "dest_port": 21,
  "proto": "TCP",
  "tx_id": 17,
  "ftp": {
    "command": "RETR",
    "command_data": "index.html",
    "reply": [
      "Opening BINARY mode data connection for index.html (6712 bytes)",
      "Transfer complete"
    ],
    "completion_code": "150"
  }
}
{
  "timestamp": "2019-03-10T15:20:21.104835-0700",
  "flow_id": 274790086810710,
  "pcap_cnt": 125,
  "event_type": "ftp",
  "src_ip": "2a01:0e34:ee97:b130:8c3e:45ea:5ac6:e301",
  "src_port": 38970,
  "dest_ip": "2001:0700:0100:0118:0000:0000:0000:0047",
  "dest_port": 21,
  "proto": "TCP",
  "tx_id": 18,
  "ftp": {
    "command": "QUIT",
    "reply": [
      "Goodbye"
    ],
    "completion_code": "221"
  }
}

Sample ftp_data event_type output:

{
  "timestamp": "2019-03-10T15:20:01.500157-0700",
  "flow_id": 1615446949360413,
  "parent_id": 274790086810710,
  "pcap_cnt": 68,
  "event_type": "ftp_data",
  "src_ip": "2a01:0e34:ee97:b130:8c3e:45ea:5ac6:e301",
  "src_port": 53697,
  "dest_ip": "2001:0700:0100:0118:0000:0000:0000:0047",
  "dest_port": 51093,
  "proto": "TCP",
  "tx_id": 0,
  "ftp_data": {
    "filename": "uio-app.js",
    "command": "RETR"
  }
}
{
  "timestamp": "2019-03-10T15:20:18.000469-0700",
  "flow_id": 1343493916181127,
  "parent_id": 274790086810710,
  "pcap_cnt": 116,
  "event_type": "ftp_data",
  "src_ip": "2001:0700:0100:0118:0000:0000:0000:0047",
  "src_port": 20,
  "dest_ip": "2a01:0e34:ee97:b130:8c3e:45ea:5ac6:e301",
  "dest_port": 41813,
  "proto": "TCP",
  "tx_id": 0,
  "ftp_data": {
    "filename": "index.html",
    "command": "RETR"
  }
}

@jlucovsky
eve: Log FTP transactions
1aa8da3 
This changeset includes changes that
1. Add transaction support to the FTP parser
2. Support eve json logging of FTP transactions
@jlucovsky
Add option for ftp logging to eve-log
ce05cb1
@jlucovsky jlucovsky requested review from norg and a team as code owners May 4, 2019 14:47
@jlucovsky jlucovsky mentioned this pull request May 4, 2019
Closed
victorjulien
victorjulien requested changes May 5, 2019
View reviewed changes
Copy link
Member

@victorjulien victorjulien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No review: just some QA results.

src/app-layer-ftp.c

tx_complete:
tx->done = true;
return retcode;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

  CC       app-layer-ftp.o
app-layer-ftp.c:803:16: warning: Potential leak of memory pointed to by 'response'
    tx->done = true;
               ^~~~
/usr/lib/llvm-8/lib/clang/8.0.0/include/stdbool.h:32:14: note: expanded from macro 'true'
#define true 1
             ^
1 warning generated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproduced and fixed.

src/app-layer-ftp.c
}

tx->command_descriptor = cmd_descriptor;
tx->request_length = CopyCommandLine(&tx->request, state->current_line, state->current_line_len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

=================================================================
==18232==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1087 byte(s) in 53 object(s) allocated from:
    #0 0x7f79ef3f468e in calloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10c68e)
    #1 0x5603f516d778 in FTPCalloc /builds/inliniac/suricata-ci/suricata/src/app-layer-ftp.c:211
    #2 0x5603f516fb53 in CopyCommandLine /builds/inliniac/suricata-ci/suricata/src/app-layer-ftp.c:477
    #3 0x5603f517044e in FTPParseRequest /builds/inliniac/suricata-ci/suricata/src/app-layer-ftp.c:604
    #4 0x5603f519c23f in AppLayerParserParse /builds/inliniac/suricata-ci/suricata/src/app-layer-parser.c:1187
    #5 0x5603f50c7851 in AppLayerHandleTCPData /builds/inliniac/suricata-ci/suricata/src/app-layer.c:646
    #6 0x5603f55ffd56 in ReassembleUpdateAppLayer /builds/inliniac/suricata-ci/suricata/src/stream-tcp-reassemble.c:1070
    #7 0x5603f560029f in StreamTcpReassembleAppLayer /builds/inliniac/suricata-ci/suricata/src/stream-tcp-reassemble.c:1140
    #8 0x5603f5603900 in StreamTcpReassembleHandleSegmentUpdateACK /builds/inliniac/suricata-ci/suricata/src/stream-tcp-reassemble.c:1706
    #9 0x5603f5603ba6 in StreamTcpReassembleHandleSegment /builds/inliniac/suricata-ci/suricata/src/stream-tcp-reassemble.c:1749
    #10 0x5603f55bfa06 in HandleEstablishedPacketToClient /builds/inliniac/suricata-ci/suricata/src/stream-tcp.c:2375
    #11 0x5603f55c2478 in StreamTcpPacketStateEstablished /builds/inliniac/suricata-ci/suricata/src/stream-tcp.c:2612
    #12 0x5603f55de83b in StreamTcpStateDispatch /builds/inliniac/suricata-ci/suricata/src/stream-tcp.c:4617
    #13 0x5603f55dfec6 in StreamTcpPacket /builds/inliniac/suricata-ci/suricata/src/stream-tcp.c:4798
    #14 0x5603f55e1a5a in StreamTcp /builds/inliniac/suricata-ci/suricata/src/stream-tcp.c:5134
    #15 0x5603f543ab7a in FlowWorker /builds/inliniac/suricata-ci/suricata/src/flow-worker.c:216
    #16 0x5603f562d859 in TmThreadsSlotVarRun /builds/inliniac/suricata-ci/suricata/src/tm-threads.c:128
    #17 0x5603f562fb7f in TmThreadsSlotVar /builds/inliniac/suricata-ci/suricata/src/tm-threads.c:583
    #18 0x7f79eecb1181 in start_thread /build/glibc-KRRWSm/glibc-2.29/nptl/pthread_create.c:486

Direct leak of 885 byte(s) in 25 object(s) allocated from:
    #0 0x7f79ef3f468e in calloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10c68e)
    #1 0x5603f516d778 in FTPCalloc /builds/inliniac/suricata-ci/suricata/src/app-layer-ftp.c:211
    #2 0x5603f516fb53 in CopyCommandLine /builds/inliniac/suricata-ci/suricata/src/app-layer-ftp.c:477
    #3 0x5603f517044e in FTPParseRequest /builds/inliniac/suricata-ci/suricata/src/app-layer-ftp.c:604
    #4 0x5603f519c23f in AppLayerParserParse /builds/inliniac/suricata-ci/suricata/src/app-layer-parser.c:1187
    #5 0x5603f50c6ac2 in TCPProtoDetect /builds/inliniac/suricata-ci/suricata/src/app-layer.c:442
    #6 0x5603f50c750d in AppLayerHandleTCPData /builds/inliniac/suricata-ci/suricata/src/app-layer.c:601
    #7 0x5603f55ffd56 in ReassembleUpdateAppLayer /builds/inliniac/suricata-ci/suricata/src/stream-tcp-reassemble.c:1070
    #8 0x5603f560029f in StreamTcpReassembleAppLayer /builds/inliniac/suricata-ci/suricata/src/stream-tcp-reassemble.c:1140
    #9 0x5603f5603900 in StreamTcpReassembleHandleSegmentUpdateACK /builds/inliniac/suricata-ci/suricata/src/stream-tcp-reassemble.c:1706
    #10 0x5603f5603ba6 in StreamTcpReassembleHandleSegment /builds/inliniac/suricata-ci/suricata/src/stream-tcp-reassemble.c:1749
    #11 0x5603f55bfa06 in HandleEstablishedPacketToClient /builds/inliniac/suricata-ci/suricata/src/stream-tcp.c:2375
    #12 0x5603f55c2478 in StreamTcpPacketStateEstablished /builds/inliniac/suricata-ci/suricata/src/stream-tcp.c:2612
    #13 0x5603f55de83b in StreamTcpStateDispatch /builds/inliniac/suricata-ci/suricata/src/stream-tcp.c:4617
    #14 0x5603f55dfec6 in StreamTcpPacket /builds/inliniac/suricata-ci/suricata/src/stream-tcp.c:4798
    #15 0x5603f55e1a5a in StreamTcp /builds/inliniac/suricata-ci/suricata/src/stream-tcp.c:5134
    #16 0x5603f543ab7a in FlowWorker /builds/inliniac/suricata-ci/suricata/src/flow-worker.c:216
    #17 0x5603f562d859 in TmThreadsSlotVarRun /builds/inliniac/suricata-ci/suricata/src/tm-threads.c:128
    #18 0x5603f562fb7f in TmThreadsSlotVar /builds/inliniac/suricata-ci/suricata/src/tm-threads.c:583
    #19 0x7f79eecb1181 in start_thread /build/glibc-KRRWSm/glibc-2.29/nptl/pthread_create.c:486

SUMMARY: AddressSanitizer: 1972 byte(s) leaked in 78 allocation(s).
ERROR: Job failed: exit code 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not able to reproduce with the pcaps that I'm using. Could you share the pcaps used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was able to reproduce with provided pcap. Fixed in continuation pr.

@jlucovsky jlucovsky mentioned this pull request May 7, 2019
Closed
@jlucovsky
Copy link
Contributor Author

Continued in #3839

@jlucovsky jlucovsky closed this May 7, 2019
@jlucovsky jlucovsky deleted the 2315.3 branch August 24, 2019 16:11
catenacyber added a commit to catenacyber/suricata that referenced this pull request Jul 19, 2023
@catenacyber
output/alert: rewrite code for app-layer properties
33de692 
Especially fix setup-app-layer script to not forget this part

Ticket: OISF#3827
catenacyber added a commit to catenacyber/suricata that referenced this pull request Sep 18, 2023
@catenacyber
output/alert: rewrite code for app-layer properties
803092e 
Especially fix setup-app-layer script to not forget this part

Ticket: OISF#3827
catenacyber added a commit to catenacyber/suricata that referenced this pull request Sep 18, 2023
@catenacyber
output/alert: rewrite code for app-layer properties
d6ee22b 
Especially fix setup-app-layer script to not forget this part

Ticket: OISF#3827
catenacyber added a commit to catenacyber/suricata that referenced this pull request Sep 18, 2023
@catenacyber
output/alert: rewrite code for app-layer properties
1e77969 
Especially fix setup-app-layer script to not forget this part

Ticket: OISF#3827
catenacyber added a commit to catenacyber/suricata that referenced this pull request Nov 10, 2023
@catenacyber
output/alert: rewrite code for app-layer properties
c2693d4 
Especially fix setup-app-layer script to not forget this part

This allows, for simple loggers, to have a unique definition
of the actual logging function with the jsonbuilder.
This way, alerts, files, and app-layer event can share the code
to output the same data.

Ticket: OISF#3827
catenacyber added a commit to catenacyber/suricata that referenced this pull request Nov 16, 2023
@catenacyber
output/alert: rewrite code for app-layer properties
2efff72 
Especially fix setup-app-layer script to not forget this part

This allows, for simple loggers, to have a unique definition
of the actual logging function with the jsonbuilder.
This way, alerts, files, and app-layer event can share the code
to output the same data.

Ticket: OISF#3827
catenacyber added a commit to catenacyber/suricata that referenced this pull request Nov 16, 2023
@catenacyber
output/alert: rewrite code for app-layer properties
da33595 
Especially fix setup-app-layer script to not forget this part

This allows, for simple loggers, to have a unique definition
of the actual logging function with the jsonbuilder.
This way, alerts, files, and app-layer event can share the code
to output the same data.

Ticket: OISF#3827
victorjulien pushed a commit to victorjulien/suricata that referenced this pull request Nov 17, 2023
@catenacyber @victorjulien
output/alert: rewrite code for app-layer properties
2118e3c 
Especially fix setup-app-layer script to not forget this part

This allows, for simple loggers, to have a unique definition
of the actual logging function with the jsonbuilder.
This way, alerts, files, and app-layer event can share the code
to output the same data.

Ticket: OISF#3827
catenacyber added a commit to catenacyber/suricata that referenced this pull request Nov 19, 2023
@catenacyber
output/alert: rewrite code for app-layer properties
b48c7d7 
Especially fix setup-app-layer script to not forget this part

This allows, for simple loggers, to have a unique definition
of the actual logging function with the jsonbuilder.
This way, alerts, files, and app-layer event can share the code
to output the same data.

Ticket: OISF#3827
catenacyber added a commit to catenacyber/suricata that referenced this pull request Nov 20, 2023
@catenacyber
output/alert: rewrite code for app-layer properties
0b6b015 
Especially fix setup-app-layer script to not forget this part

This allows, for simple loggers, to have a unique definition
of the actual logging function with the jsonbuilder.
This way, alerts, files, and app-layer event can share the code
to output the same data.

Ticket: OISF#3827
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@victorjulien victorjulien victorjulien requested changes

@norg norg Awaiting requested review from norg

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jlucovsky @victorjulien